[data grid] Fix scroll jumping #19156
Conversation
romgrk
added
scope: data grid
|
Deploy preview: https://deploy-preview-19156--material-ui-x.netlify.app/ Bundle size report
|
oliviertassinari
changed the title
| @@ -259,7 +258,7 @@ function useDimensions(store: Store<BaseState>, params: VirtualizerParams, _api: | |||
| ); | |||
| React.useEffect(() => debouncedUpdateDimensions?.clear, [debouncedUpdateDimensions]); | |||
|
|
|||
| useLayoutEffect(() => observeRootNode(refs.container.current, store), [refs, store]); | |||
| useLayoutEffect(() => observeRootNode(containerNode, store), [containerNode, store]); | |||
There was a problem hiding this comment.
This change broke horizontal virtualization: https://deploy-preview-19156--material-ui-x.netlify.app/x/react-data-grid/#pro-version
There was a problem hiding this comment.
Hmm, I don't understand why it doesn't work as expected though:
const containerNode = refs.container.current;
console.log('render', containerNode);
React.useLayoutEffect(() => {
console.log('useLayoutEffect', containerNode);
return observeRootNode(containerNode, store);
}, [containerNode, store]);Logs:
render null
dimensions.ts:261 render null
dimensions.ts:264 useLayoutEffect null
dimensions.ts:527 observeRootNode null
dimensions.ts:261 render null
dimensions.ts:261 render null
dimensions.ts:261 render <div class="MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="1" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="1" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="1" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="100001" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="100001" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="100001" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="100001" aria-multiselectable="true">…</div>flex
dimensions.ts:261 render <div class="MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex="-1" role="grid" aria-colcount="30" aria-rowcount="100001" aria-multiselectable="true">…</div>flex
How come useLayoutEffect doesn't rerun when containerNode clearly changes and there are rerenders? 🤔
There was a problem hiding this comment.
I tried moving this effect to the very top of the hook to make sure that it's not cancelled by another effect that has state update inside, but the behavior is the same.
This looks like a React bug to me.
There was a problem hiding this comment.
Or a nextjs bug – I can't reproduce the issue in stackblitz using the packages built from this branch: https://stackblitz.com/edit/kygkymrj?file=package.json
There was a problem hiding this comment.
Commenting out this line makes layout effect run after containerNode changes and everything seems to work fine:
This means that the layout effect is indeed cancelled because of state update in useStoreEffect
There was a problem hiding this comment.
For some reason, the issue doesn't appear for me in the playground page, it only appears in the other docs pages.
I'll refactor the state update away from the store effect.
There was a problem hiding this comment.
I can reproduce it on playground page:
There was a problem hiding this comment.
How's your playground page setup? Mine works like this, which has been reported (by others) to avoid some nextjs issues, maybe I should configure it differently.
There was a problem hiding this comment.
I can confirm that if I remove the shell that I have in my playground page, I observe the same issues that are otherwise present only in the rest of the pages.
Closes #19017
The effect was running too often because the container wasn't stable. Adding/removing the
ResizeObserverseems to cause scroll jumps on firefox (and maybe chrome).